Repository navigation
refactor: SimulationCfg.play_retargeted_keys() (ROADMAP §9 PR R3.2 / ADR-0005) - #91
Merged
Merged
Conversation
… (ROADMAP §9 PR R3.2 / ADR-0005)
Move the play-mode shortcut-retargeting key list off the private
`cli/__init__.py:_PLAY_RETARGETED_KEYS` constant onto
`SimulationCfg.play_retargeted_keys()` (static method on the domain
config). The CLI's `env.` → `play_env.` retarget loop in play mode
now calls the method; the set of play-retargetable simulation override
paths lives next to the SimulationCfg fields the --vis / --gpu /
--steps / --dt shortcuts target.
Returns the four paths verbatim (`env.simulation.{vis,gpu,steps,dt}`)
so the CLI use-site is unchanged except for the constant → method swap
— the lowest-risk faithful replacement per ADR-0005's framing. All
four are real SimulationCfg fields (test_configs.py asserts this).
Completes ADR-0005 / R3 (R3.1 EvalCallbackCfg.from_args shipped in
PR #90; this is R3.2).
New `tests/test_configs.py` (3 tests): exact ordered set, every key
names a real SimulationCfg field, and class/instance call equivalence.
Reference audit (per CLAUDE.md §9.1 rule 1): `_PLAY_RETARGETED_KEYS`
had a single use site (the play-mode retarget loop); removed.
`SimulationCfg` added to the existing module-top
`from genelab.configs import …` line in cli/__init__.py (cli → configs
is the allowed layering direction).
Verified: ruff ✓, pyright 0/0/0, full suite 395 passed (was 392; +3
from test_configs.py), `genelab play --help` snapshots byte-identical
(R0.1 gate), configs.py stays torch-free at import (invariant #5),
lint-imports baseline unchanged at 2 kept / 2 broken.
cli/__init__.py LoC: 1051 (refactor start) → 1020 after R3.1 + R3.2
(−31). ADR-0005 §10 estimated ≥35; the actual parse logic was a touch
smaller. Both parsers (eval-callback args, play-retarget keys) are now
domain-owned, which was the substantive goal.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
KraHsu
added a commit
that referenced
this pull request
Jun 5, 2026
refactor: SimulationCfg.play_retargeted_keys() (ROADMAP §9 PR R3.2 / ADR-0005)
KraHsu
added a commit
that referenced
this pull request
Jun 5, 2026
…erged) Mark Phase R3 (domain-owned parsing, ADR-0005) complete (PRs #90/#91) and Phase R4 (CLI decomposition, ADR-0004) in progress with R4.1 merged (PR #92). Records the R3.1/R3.2/R4.1 shipped entries, test results, the unchanged 2-kept/2-broken/21 importlinter baseline, and the new __all__ lesson from R4.1. Corrects the prior wrong claim that R3 would clear the `rl.eval_callback -> cli._eval` violation: R3 moved a parser in the allowed cli->rl direction; the violation is the rl->cli runtime import of eval_task inside run_with_eval_callback, untouched by R3. No R-phase as currently scoped reduces the baseline — flagged as a dedicated follow-up gating R7. Sets R4.2 (cli/_multi_seed.py) as the next slice. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cli/__init__.py:_PLAY_RETARGETED_KEYSontoSimulationCfg.play_retargeted_keys().genelab play --helpbyte-identical (R0.1 gate);configs.pystays torch-free (invariant ci/per job cache suffix and pages enablement #5).What changed
src/genelab/configs.pySimulationCfg.play_retargeted_keys() -> tuple[str, ...]static method (+19 LoC). Returns the fourenv.simulation.{vis,gpu,steps,dt}override paths verbatim.src/genelab/cli/__init__.pySimulationCfg.play_retargeted_keys();_PLAY_RETARGETED_KEYSconstant removed;SimulationCfgadded to the existing module-topfrom genelab.configs import …. Net −10.tests/test_configs.py(new, 3 tests)SimulationCfgfield; class/instance call equivalence.CHANGELOG.md[Unreleased] · Changedentry.Design decision (confirmed with maintainer)
Returns the full paths verbatim (
env.simulation.*) rather than bare field names — faithful to ADR-0005's "replaces_PLAY_RETARGETED_KEYS" framing, leaving the CLI use-site unchanged except for the constant → method swap. The minor smell (aSimulationCfgmethod referencing itsenv.simulation.mount path) was accepted over the alternative (bare names + CLI composes the prefix), which would have changed the use-site logic.Reference audit (per CLAUDE.md §9.1 rule 1)
_PLAY_RETARGETED_KEYShad a single use site — the play-modeenv.→play_env.retarget loop. Removed.SimulationCfgadded to the existingfrom genelab.configs import apply_overridesline (nowimport SimulationCfg, apply_overrides);cli → configsis the allowed layering direction.Local verification
uv run pytest tests/test_configs.py— 3 passeduv run pytest tests/test_cli_help_snapshots.py— 11 passed (byte-identical) — R0.1 gate confirms no--helpchangepython -c "import sys; import genelab.configs; assert 'torch' not in sys.modules"— configs.py torch-free (invariant ci/per job cache suffix and pages enablement #5)uv run ruff check && uv run ruff format --check— All checks passeduv run pyright— 0 errors, 0 warnings, 0 informationsuv run pytest— 395 passed (was 392; +3 from the new test)uv run lint-imports—Contracts: 2 kept, 2 broken(unchanged —cli → configs, allowed direction)ADR-0005 §10 completion criteria
EvalCallbackCfg.from_args(5 tests), R3.2SimulationCfg.play_retargeted_keys(3 tests)cli/__init__.pyshrinks ≥ 35 LoC--eval-*args, play-retarget keys) now domain-owned — is met.Phase R3 complete on merge
EvalCallbackCfg.from_args6b97f6e)SimulationCfg.play_retargeted_keysR3 was the gate for R4 (CLI decomposition), which can now proceed (serial after R3 per §9.2).
Note on the importlinter baseline
Like R3.1, R3.2 is
cli → configs(allowed direction) and does not reduce the importlinter baseline (still 2 kept / 2 broken). Therl.eval_callback → cli._evalviolation remains a separate follow-up (moveeval_task's domain core intorl/).Rollback
git revert <merge-sha>. Restores_PLAY_RETARGETED_KEYSincli/__init__.pyand removes the method + test.🤖 Generated with Claude Code